-
Notifications
You must be signed in to change notification settings - Fork 157
Organize mingw includes #1985
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Organize mingw includes #1985
Conversation
We want to make them relative to the top-level directory. Signed-off-by: Johannes Schindelin <[email protected]>
It allows for more consistent patches that way. Signed-off-by: Johannes Schindelin <[email protected]>
/submit |
Submitted as [email protected] To fetch this version into
To fetch this version to local tag
|
On the Git mailing list, Patrick Steinhardt wrote (reply to this): On Thu, Oct 09, 2025 at 07:45:59AM +0000, Johannes Schindelin via GitGitGadget wrote:
> Following in the footsteps of the many, many recent #include refactorings,
> this patch series orders the #include statements in compat/mingw.c.
Both of these patches look good to me and I like the improved
consistency that they bring. I may also do the same for our code in
"refs/", where some of the files use relative includes, as well. Might
be worth to document this somewhere if it isn't already, but that
doesn't have to be part of your patch series here.
Sorting them also makes sense. It's another thing where I wish that we
had a tool to enforce this. clang-format supports this in theory, but
it's disabled right now. And I'm not even sure whether it can be told to
include e.g. "git-compat-util.h" first.
Thanks!
Patrick |
User |
On the Git mailing list, Johannes Schindelin wrote (reply to this): Hi Patrick,
On Fri, 10 Oct 2025, Patrick Steinhardt wrote:
> On Thu, Oct 09, 2025 at 07:45:59AM +0000, Johannes Schindelin via GitGitGadget wrote:
> > Following in the footsteps of the many, many recent #include refactorings,
> > this patch series orders the #include statements in compat/mingw.c.
>
> Both of these patches look good to me and I like the improved
> consistency that they bring. I may also do the same for our code in
> "refs/", where some of the files use relative includes, as well. Might
> be worth to document this somewhere if it isn't already, but that
> doesn't have to be part of your patch series here.
There's already such a lot of documentation that I fear it is intimidating
to any new contributor (and if I was one, I'd be tempted to read it
exclusively via AI summaries). I don't want to add to that amount.
> Sorting them also makes sense. It's another thing where I wish that we
> had a tool to enforce this. clang-format supports this in theory, but
> it's disabled right now. And I'm not even sure whether it can be told to
> include e.g. "git-compat-util.h" first.
In theory, I am totally with you: Sorting `#include`s is a job best left
to tools. But then, I say the same about formatting, and I see a lot of
appetite on this list for preventing that to become a tool-driven job, and
instead requiring developer time to be spent on it. Therefore, I don't
want to spend any effort trying to get this automated when I see small
chances of success in the endeavor of freeing up cognitive cycles for
tasks I find more intellectually rewarding than figuring out where a space
or a line break goes.
Ciao,
Johannes |
On the Git mailing list, Junio C Hamano wrote (reply to this): Johannes Schindelin <[email protected]> writes:
>> Sorting them also makes sense. It's another thing where I wish that we
>> had a tool to enforce this. clang-format supports this in theory, but
>> it's disabled right now. And I'm not even sure whether it can be told to
>> include e.g. "git-compat-util.h" first.
>
> In theory, I am totally with you: Sorting `#include`s is a job best left
> to tools. But then, I say the same about formatting,
I am afraid that it is apples-to-oranges comparison. Nobody has to
read the #include directives; they may have to see if a header they
care about (because they are planning to add a call to a function
that hasn't been used in the particular C source file, perhaps) is
already included, and sorted list of includes is a good tool to help
them. IOW, they do not read them, they scan in them. But the code,
the result of formatting, must be readable by people. |
This patch series was integrated into seen via git@4bf6293. |
Following in the footsteps of the many, many recent
#include
refactorings, this patch series orders the#include
statements incompat/mingw.c
.cc: Patrick Steinhardt [email protected]